Breakpoint improvements #1809
Breakpoint improvements #1809
Conversation
rocketz
commented
Nov 24, 2024
rocketz
commented
- Cleaned up UI. New breakpoints are now handled in a pop-up dialog
- Breakpoint are now shown in a table.
- You can easily see and edit breakpoint names in the table
- Clicking on the address will take you there (in asm or memory view)
- Enable all/disable all/delete all buttons
- Context menu in assembly view to quickly create read/write breakpoints on the address the instruction reads/writes
- New breakpoint types:
- On changes. WIll only break when the value is written to with a change. The new value written is the new value tested against
- Lower than
- Higher than
- Equal
- Range
- When a code breakpoint is hit it is highlighted in red in breakpoint table
- Cleaned up UI. New breakpoints are now handled in a pop-up dialog - Breakpoint are now shown in a table. - You can easily see and edit breakpoint names in the table - Clicking on the address will take you there (in asm or memory view) - Enable all/disable all/delete all buttons - Context menu in assembly view to quickly create read/write breakpoints on the address the instruction reads/writes - New breakpoint types: * On changes. WIll only break when the value is written to with a change. The new value written is the new value tested against * Lower than * Higher than * Equal * Range - When a code breakpoint is hit it is highlighted in red in breakpoint table
| #include "fmt/format.h" | ||
| #include "imgui.h" | ||
| #include "support/imgui-helpers.h" | ||
|
|
||
| static ImVec4 s_currentColor = ImColor(0xff, 0xeb, 0x3b); | ||
| // Note: We ignore SWL and SWR |
There was a problem hiding this comment.
This is a bit of a blind spot overall. It feels like we can properly support swl/swr. I'll try and work this out a bit.
| MemVal final = {}; | ||
| switch (width) { | ||
| case 1: { | ||
| uint8_t val = PCSX::g_emulator->m_mem->read8(addr); |
There was a problem hiding this comment.
We shouldn't use read8/read16/read32 for the purpose of the debugger, as it may advance internal state machines on things like the cd-rom controller or the pads. This one's a bit difficult.
There was a problem hiding this comment.
oh I see.. maybe we need a more direct access?
src/gui/widgets/breakpoints.cc
Outdated
| doBreak = curVal != self->conditionData(); | ||
| if (doBreak) | ||
| { | ||
| // TODO: can't update since 'self' is const |
There was a problem hiding this comment.
Could you fill in what'd need to be updated still? It's probably doable to make everything work properly still. It may be fine to also change the upper API generally speaking, but I'd rather we discuss this a bit. We could also use mutable within reason.
There was a problem hiding this comment.
I actually have this working, but I messed up with some stashes. Will bring those changes in.
src/gui/widgets/breakpoints.cc
Outdated
| // than that they want to use the same label twice | ||
| m_bpLabelString[0] = 0; | ||
|
|
||
| static int breakCondition2 = 0; |
There was a problem hiding this comment.
Could we name this breakConditionImGuiValue instead or something? The 2 suffix doesn't really explain why it's there in the first place.
My pleasure!
No worries. You shouldn't waste valuable turkey-time on this :)
Cool. I'll fix a few things though. |
|
Updating from main as there was some CI breakages, and we can see better where we're at here. |
|
Right so there's still some breakages on Linux: src/gui/widgets/breakpoints.cc:242:32: error: format not a string literal and no format arguments [-Werror=format-security]Needs to use |
